Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug fix upsampling - fixes #26 #27

Merged
merged 2 commits into from
Aug 14, 2023
Merged

Bug fix upsampling - fixes #26 #27

merged 2 commits into from
Aug 14, 2023

Conversation

tremblap
Copy link
Member

@tremblap tremblap commented Aug 13, 2023 •
edited
Loading

The condition in which more than one pixel draws from a single sample was not right. fav.js has also been updated. Fixes bug #26

@tremblap tremblap requested review from jamesb93 and weefuzzy August 13, 2023 16:52
@tremblap
Copy link
Member Author

test code:


----------begin_max5_patcher----------
682.3ocuW1raiBCDG+L7TXw4rQXvPf8TeOVspxDLYIhujsIMqpZe1WyXHIsM
AbSX4.j3ggL923Y9amWssbRpOxDNneh9Exx5UaKKvTmAq9wVNkziaKnBvMmR
lPP2wbVoeljcTB12Vvn7AqUsk0sxBlDdGbu07Tvy5j8+.GO3p1O4eaX5YgiC
528OpgJ29m7pcOyYak5mFF4s1cEJdS2c+ft6dJKmdEUfyqFhqWms2rs6tsxP
7pXunleegtrh17z0E4BoWRaF5oTlPlWQk40UHo56dlSd3MIW8Kmw3ixO1OFX
VmFvalHCfmuLfdx8tlVzSBZYi.Ebcr8tB1A2D6rhZpzYkJDzpciRuuKFV2wQ
.89gyO82p7NiQksblnmeNK07Ubx8UqGr4hZcrq27WreKXegdfkUyKAXMmS78
wIgDelyH+4Gy8h1bioHx4zLkSKYRF+YVEMo.PwcZ.yxKT9WxtP1XHc9958Bm
IyC8s3A5Q9ceLek2mlJWOeD907wlo6aG4KcJlceNtnlGzVSH509.nsNjbA2Z
wmSo0OUVdNkD7+Qu66K2ENCpcXhdWtXhtrvcwz5knDThwrdakc.QiPECaj46
oq48VLRmZecikMHOzt58h6APC.NLZ41WyE4t1ScQTWgpqHiI1+9T6w8pbwZQ
+k6Hbp9tTcAoo6mMdc83GSyU2yB3E7MVOAOTxlUe9T4PT5r+QxE0s7sCyqgS
ViNGnKJnuvoHsOWM4ZZfBLHNcG84gCDwf.ELCww2DfB+fS07ztVa0ACdnHiI
K3ZFdJDwyPfvF.j+BslQt9RF9w.DeWKY51aZSyAFWz6LDCkD19ZXdEsBFlWo
GF.C4rC4C9qsP4JQGoRwQ8uR.Iqig58fbJqUDVoNxqFREcpPBxicGfRzP0f.
pn1uY+O.T8Ovr.
-----------end_max5_patcher-----------

@tremblap
Copy link
Member Author

Overnight I had a question most probably for @weefuzzy as the main author of this codebase: now that fav.js has a proposed fix and that the code will be common, is there a rationale to keep this part separate or should we 'import' it like we do for the rest?

@weefuzzy
Copy link
Member

is there a rationale to keep this part separate or should we 'import' it like we do for the rest?

🤷 try it and see. The code do it is there and commented out...IIRC, the rationale was that the functions were different.

I don't have the wherewithal to look in detail the this fix and flucoma/fav.js#8 this week (lots to do). If you're convinced it gives the proper behaviour everywhere, then sure.

Please try not to bundle unrelated commits into PRs (67bfefd), to help with the sense-making.

@tremblap
Copy link
Member Author

ok I'll just merge now. it works for everyone who tested it. if fav gets fixed and approved we can make the merge of that later.

I was certain I had pushed to main directly the .gitignore fix, sorry

@tremblap
Copy link
Member Author

thanks for the quick reply and good luck with the heavy week.

@tremblap tremblap merged commit 5d2ecf3 into main Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesb93 jamesb93 Awaiting requested review from jamesb93

@weefuzzy weefuzzy Awaiting requested review from weefuzzy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tremblap @weefuzzy